Skip to content

Resolve Maven pin properties through profiles and nesting - #2020

Draft
Mikola Lysenko (mikolalysenko) wants to merge 1 commit into
fix/maven-discovery-false-attestationfrom
fix/maven-discovery-property-overrides
Draft

Mikola Lysenko (mikolalysenko) wants to merge 1 commit into
fix/maven-discovery-false-attestationfrom
fix/maven-discovery-property-overrides

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #2005, including its review fix (9765178). This closes the two Maven false-attestation gaps #2005 left open in VEX discovery.

1. Profile <properties> redefining a pin's property

Take a live <version>${v}</version> whose build resolution is the Socket pin. If any profile's <properties> redefine v, or any property v names, Maven resolves something else while that profile is active: it merges active profile properties over the pom's own before it interpolates. Vendored mode refuses to pin over such a property (version_override), but discovery attested it.

  • PomScope now reads each profile's <properties>.
  • Each PomDep gains version_alternatives: the other versions its build_version can take under any profile, or any set of profiles active together.
    • User properties and project.* / pom.* model references stay fixed.
    • A value that some set leaves unresolved, cyclic or too deep counts as the text as written.
  • A new resolutions(d, in_profile) helper lists every version the build may resolve for a declaration: its build_version, then each alternative. It plugs into Stop attesting Maven pins a profile or user property overrides #2005's helpers in three places:
    • unpatched_classifier_copy: live and profile classifier copies and their classifier-keyed management.
    • profile_override, profile declarations: every version they can take is checked against the pin.
    • profile_override, live declarations: it now also walks live main-jar declarations, with the same main_jar_declarations scope and management-governs filter. A declaration whose build_version is the pin but has an alternative off it keeps the ref, marked unattested with the existing UnattestedKind::MavenProfileUnpatched. This applies to hosted pins and the pre-v5 vendored wiring alike. No new variant was needed; the variant doc, the CLI note text and CLI_CONTRACT.md are updated.
  • A <profiles> declaration's own ${v} reads its own profile's <properties> first.

2. Property versions resolved one level deep

${a} with <a>${b}</a> now resolves through every property it names, in both views PomDep keeps since #2005:

  • version (as the pom spells it, which hosted writes and rolls back): the model, then the declaration's own profile's <properties>, then the root <properties>.
  • build_version (as the build resolves it): .mvn/maven.config -D user properties first, at every level. project.* / pom.* stay the model's.

It follows the vendored reactor's limits: at most 16 levels, and a cycle, an undefined name or an unterminated ${ never resolves. The version then stays as written, which is never a pin. Partly literal versions (${a}.${b}-x) resolve too, and ${project.version} resolves from the model in both views. #2005's MavenConfigOverride check (spelled vs built effective versions) therefore also catches a user property that redefines a nested property.

Shared interpolator

The vendored reactor's interpolator moves to formats/maven (still pure). That covers both interpolate_by and the duplicate body of Reactor::interpolate_with.

  • It is now interpolate_strict, a single-valued wrapper over interpolate_each. interpolate_each enumerates every value a name can take, capped at 16 results.
  • The reactor calls it with its own chain-walking lookup, and Reactor::interpolate drops its depth argument, which was always zero.
  • New in both modes: explicit cycle detection, plus work bounds of 1024 lookups and 4096 bytes. A pom whose properties each name the next one many times over now fails closed instead of expanding exponentially.
  • Vendored behavior is otherwise unchanged.

Tests

Each of these fails before this change:

  • vex::discover::maven::tests::profile_properties_redefining_a_pin_property_are_not_attested covers the base release, a nested property, an undefined value, a self-cycle, two profiles that move the pin only together, a managed pin, and a user property that wins over every profile.
  • vex::discover::maven::tests::profile_declaration_reads_its_own_profile_properties
  • vex::discover::maven::tests::version_property_resolves_through_nested_properties covers a 16-deep chain (resolves), a 17-deep chain (does not), two cycles and nested user overrides.
  • vex::discover::maven::tests::vendored_ref_under_profile_properties_is_not_attested
  • formats::maven::tests::{version_properties_resolve_recursively_and_fail_closed, profile_properties_give_version_alternatives, interpolation_is_bounded}. These also assert that a nested user property moves build_version but not version.

Changed expectation after the rebase. In version_property_resolves_through_nested_properties, the case where the pom resolves the base release and -Dp1=<pin> in .mvn/maven.config supplies the pin through a nested property now expects no attestation. Under #2005's review fix, a Socket version that only maven.config supplies is not pom.xml wiring: hosted never writes one and rollback / remove never edit .mvn/maven.config. It is diagnosed (patched_ref_unattributable), never a ref. Nested resolution now applies that rule to nested properties too.

Runs, on the rebased branch (ec89bd9):

  • cargo test -p socket-patch-core --lib -- vex::discover:: formats::maven vendor::jvm:: passes 715. patch::redirect:: together with the golden test passes 872.
  • The golden corpus is unchanged (committed_fixture_corpus_matches_golden passes).
  • cargo test -p socket-patch-cli --lib vex_sources passes 14.
  • cargo clippy --locked --workspace --all-features -- -D warnings and cargo fmt --all -- --check are clean.

Not done

🤖 Generated with Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: merge conflict needs a decision (left unmerged, nothing pushed).

This PR now conflicts with its base fix/maven-discovery-false-attestation (#2005) after 9765178 ("Address review: Maven discovery bypass checks match Maven precedence"). Both sides changed the same version-resolution behavior in incompatible ways, in formats/maven/mod.rs (6 hunks), vex/discover/maven.rs (3 hunks) and CLI_CONTRACT.md (2 hunks):

  • Stop attesting Maven pins a profile or user property overrides #2005 (base) splits PomDep::version (one-level, root <properties> only — what the hosted rewriter writes and rolls back) from a new build_version (user properties first, project.*/pom.* exempt), and reports a pin that .mvn/maven.config moves under a new UnattestedKind::MavenConfigOverride (vex_maven_config_override). It also narrows profile <dependencyManagement> to "only where management governs the pin" and drops the profile-classifier case from MavenProfileUnpatched (moved to MavenClassifierUnpatched).
  • This PR makes PomDep::version the full strict interpolation (user properties, model, own profile, root; 16 deep) with version_text and version_alternatives, adds fixed / profile_properties to PomScope, and folds user-property and profile-property moves into MavenProfileUnpatched.

Question: should this PR be rebuilt on #2005's split — keep version = pom-spelled for the rewriter, put the strict/recursive interpolation into build_version (plus version_alternatives from profile <properties>), keep MavenConfigOverride for .mvn/maven.config moves and use MavenProfileUnpatched only for profile-driven moves? Or should #2005's split be replaced by this PR's single version? Once you choose, the conflict can be resolved mechanically. Since #2026 is stacked on this branch, it may be simpler for the author's own stack tooling to restack it.


Generated by Claude Code

Closes the two Maven false-attestation gaps #2005 left open in VEX
discovery.

1. Profile <properties> redefining a pin's property. A live
   <version>${v}</version> that the root <properties> resolve to the
   Socket pin resolves something else whenever an active profile's
   <properties> redefine v (or a property v names): Maven merges active
   profile properties over the pom's own before interpolating. Vendored
   mode refuses to pin over such a property (version_override); discovery
   attested it. PomScope now reads each profile's <properties>, and every
   PomDep carries version_alternatives: the other versions its
   build_version takes under any profile or set of profiles active
   together (user properties and project.* model references stay fixed; a
   value some set leaves unresolved, cyclic or too deep counts as the text
   as written). profile_override now walks live main-jar declarations too
   (same main_jar_declarations scope and management-governs filter): one
   whose build version is the pin but has an alternative off it marks the
   ref unattested with the existing UnattestedKind::MavenProfileUnpatched
   (CLI note and CLI_CONTRACT.md updated), in hosted and the pre-v5
   vendored wiring alike. Profile declarations and the shared classifier
   check (unpatched_classifier_copy) count every alternative as a version
   the declaration may resolve. A <profiles> declaration's own ${v} reads
   its own profile's <properties> first.

2. Property versions resolved one level deep. ${a} with <a>${b}</a> now
   resolves through every property it names, in both views PomDep keeps:
   `version` (as the pom spells it: model, own profile, root properties)
   and `build_version` (.mvn/maven.config user properties first at every
   level; project.* stays the model's). Limits are the vendored reactor's:
   at most 16 levels, and a cycle, undefined name or unterminated ${ never
   resolves (the version stays as written, which is never a pin). A partly
   literal version (${a}.${b}-x) resolves too, and ${project.version}
   resolves from the model in both views.

The vendored reactor's interpolator (interpolate_by and the duplicate
body of Reactor::interpolate_with) moves to formats/maven as the pure
interpolate_strict, built on interpolate_each, which enumerates the
values a name may take (capped at 16 results). Both now detect cycles
explicitly and bound work (1024 lookups, 4096 bytes), so a pom whose
properties each name the next many times over fails closed instead of
expanding exponentially. Reactor::interpolate drops its always-zero
depth argument. Vendored behavior is otherwise unchanged.

Tests (each fails before this change):
- vex::discover::maven::tests::profile_properties_redefining_a_pin_property_are_not_attested
- vex::discover::maven::tests::profile_declaration_reads_its_own_profile_properties
- vex::discover::maven::tests::version_property_resolves_through_nested_properties
  (16-deep chain resolves, 17 does not; two cycles; nested user overrides)
- vex::discover::maven::tests::vendored_ref_under_profile_properties_is_not_attested
- formats::maven::tests::{version_properties_resolve_recursively_and_fail_closed,
  profile_properties_give_version_alternatives, interpolation_is_bounded}
The golden corpus is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mikolalysenko
Mikola Lysenko (mikolalysenko) force-pushed the fix/maven-discovery-property-overrides branch from d7b4b40 to ec89bd9 Compare November 4, 2026 21:39
Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Nov 4, 2026
The base branch was force-pushed (review fixes added; MavenConfigOverride
split out of profile overrides, build_version beside version, classifier
handling moved into unpatched_classifier_copy). Resolved against the old
base commit d7b4b40 so only this PR's changes are re-applied:

- coordinate_override now runs from bypasses after profile_override, and
  reads build_version (as the base's live-declaration checks do) and
  is_executable_classifier instead of the removed list.
- coordinate_alternatives reads the no-profile baseline with user
  properties (lookup's new build flag), matching lookup_any.
- CLI_CONTRACT and the profile note keep both sides' wording.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011f9aePp9J8NPBFw7TPjhfT
Mikola Lysenko (mikolalysenko) added a commit that referenced this pull request Nov 5, 2026
Closes the last Maven property false-attestation gap in VEX discovery
after #2020: a <groupId> / <artifactId> written as ${prop} that a
profile's <properties> redefine.

Coordinates were read only through the lenient interpolator, from the
root <properties> (with .mvn/maven.config user properties over them),
so discovery attested two shapes the build may not resolve:

(a) The declaration carrying the pin names another package whenever a
    profile redefining its coordinate property is active; the pin then
    no longer applies to the patched GA, which resolves from its other
    declarations or transitively, unpinned.
(b) Another declaration (live, or inside a profile whose own properties
    retarget it) becomes the patched GA under a profile, at its own
    version off the pin.

formats/maven: interpolate_each_keeping is interpolate_each with the
lenient reading's semantics (an undefined name, cycle, chain past 16
levels or unterminated ${ stays as written, so may_name reads it as any
text; None only past the work bounds). PomDep now carries
coordinate_alternatives: every (groupId, artifactId) pair the
declaration may name under any profile or set of profiles (its own
profile's properties first for a profile declaration), minus what it
reads with no profile active. User properties and project.* model
references stay fixed, as for versions. ScopedDep.group / artifact
(the lenient base the hosted rewriter also reads) are unchanged; they
gain the raw texts.

vex/discover/maven: bypasses() also runs coordinate_override (after
profile_override), reported as the existing MavenProfileUnpatched, for
(a) the live main-jar declaration carrying the pin (build_version ==
pin) with any alternative, and (b) any declaration with an alternative
that may be the GA's main jar (as main_jar_declarations reads one:
literal, or may_shadow; pom / sources / javadoc types skipped) at a
version other than the pin among its resolutions(), or literally an
executable classifier copy off the pin or with no version of its own.
As in profile_override, a managed declaration counts only where
management governs the pin, and a main jar without a version takes the
live pin and does not withhold. Applies to hosted and the pre-v5
vendored wiring. Coordinates resolve in the build's view (user
properties first). CLI note and CLI_CONTRACT.md updated.

vendor/jvm/maven_reactor: the v5 vendored planner had the same gap.
coordinates_elsewhere enumerates inheritors and every profile on the
chain (one at a time, as version_override does). (a) The existing
coordinates_overridden check now weighs profiles too, not only
inheritors: a declaration a profile moves off the GA is left as is and
its root unpinned. (b) New warn_moved_coordinates: a declaration that
names another artifact in its pom but the GA in an inheritor or under a
profile is warned (coordinates_overridden) and, when versioned, unpins
its root and is uncertain, so attestable_checked no longer attests it.
This also covers the inheritor form of (b), which was likewise unseen.

Tests (each fails before this change):
- vex::discover::maven::tests::profile_properties_moving_the_pinned_coordinates_are_not_attested
- vex::discover::maven::tests::profile_properties_turning_a_declaration_into_the_pinned_ga_are_not_attested
- vendor::jvm::maven_reactor::tests::a_profile_moving_the_declared_coordinates_is_left_alone
- vendor::jvm::maven_reactor::tests::a_profile_turning_a_declaration_into_the_patched_artifact_is_weighed
New API: formats::maven::tests::{keeping_interpolation_leaves_unresolved_references,
profile_properties_give_coordinate_alternatives}.
Guard: vex::discover::maven::tests::unrelated_coordinate_properties_do_not_withhold.
User-property variants (a chain through -D, -D beating the profile) are
in each. The golden corpus is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant